fix: race condition among transactions when running parallely #2369

alkatrivedi · 2025-08-04T06:26:38Z

surbhigarg92 · 2025-08-04T11:32:54Z

system-test/spanner.ts

+          process.env.GOOGLE_CLOUD_SPANNER_MULTIPLEXED_SESSIONS = 'true';
+          process.env.GOOGLE_CLOUD_SPANNER_MULTIPLEXED_SESSIONS_FOR_RW = 'true';


Why do we need this condition, we have 2 separate pipeline for mux and non-mux.

yeah, it was for local testing. I have removed it

surbhigarg92 · 2025-08-04T11:43:32Z

system-test/spanner.ts

+            while (sync.count !== sync.target) {
+              // wait for 500 milliseconds
+              await new Promise(resolve => setTimeout(resolve, 500));
+            }


You need to add a wait after you receive precommittoken, so dont use mutation only case but execute a read before the commit

thanks for this catch, I have updated the test to execute a read before mutations. now we will have a precommit token while committing the transactions

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

sakthivelmanii · 2025-08-05T17:24:27Z

protos/protos.d.ts

        /** Edition enum. */
        enum Edition {
            EDITION_UNKNOWN = 0,
+            EDITION_LEGACY = 900,


What are these changes?

owlbot is pushing these changes, these are not manual

system-test/spanner.ts

sakthivelmanii · 2025-08-05T17:35:32Z

system-test/spanner.ts

+          await Promise.all(promises);
+        });
+
+        it('POSTGRESQL should insert and commit transactions when running parallely', async () => {


why do we have to write this specifically for PG_DIALECT? There's no difference between dialects here right?

yeah, I have removed it

sakthivelmanii · 2025-08-05T17:36:52Z

test/multiplexed-session.ts

    });

    it('should pass back the session and txn', done => {
      const fakeTxn = new FakeTransaction() as unknown as Transaction;


where are we mocking session.!transaction method? Ideally this test should fail since we removed the assignment right?

we have removed the assignment but we are returning the transaction using transaction method defined in Session class.

session!.transaction((session!.parent as Database).queryOptions_),

sakthivelmanii · 2025-08-05T17:37:41Z

test/spanner.ts

+          );
+
+          // assert that seqNum is different in both the commit request
+          assert.notEqual(


why do you expect seqNum to be different?

I guess because mockspanner return a random number for sequence number , randomInt(1, 1000),

But thats not a valid assertion. SeqNum may or maynot be same for 2 transactions running in parallel.

I am not sure if the seqNum is supposed to be an incremental number from 1...n . If yes, then mockSpanner should have followed the same logic.

I am not sure if the seqNum is supposed to be an incremental number from 1...n

yes, I have updated the mock spanner logic and hence, the test case to run read before performing mutations. now we will be sure that precommit token is there for both the transactions while making commit request.

surbhigarg92 · 2025-08-06T03:54:38Z

system-test/spanner.ts

        commitTransaction(done, PG_DATABASE, postgreSqlTable);
      });

+      describe('when multiplexed session is enabled for read write', async () => {


This need not be specific to multiplexed session, the scenario we are trying to test here is that parallel transaction should work , for both mux and non-mux. So describe should mention that.

yeah, correct

I have updated the test name, it will anyway will be running against both the pipelines, one with mux enabled and another with mux not enabled

test/spanner.ts

test/mockserver/mockspanner.ts

surbhigarg92 · 2025-08-08T05:37:26Z

test/spanner.ts

            precommitToken: {
              precommitToken: fakeRetryToken,
-              seqNum: 1,
+              seqNum: 2,


Suggested change

seqNum: 2,

// Retry should have a higher sequence number

seqNum: 2,

test/mockserver/mockspanner.ts

alkatrivedi requested review from a team as code owners August 4, 2025 06:26

product-auto-label bot added size: s Pull request size is small. api: spanner Issues related to the googleapis/nodejs-spanner API. size: xl Pull request size is extra large. and removed size: s Pull request size is small. labels Aug 4, 2025

alkatrivedi force-pushed the fix-race-condition branch from 314783f to b7eac5f Compare August 4, 2025 08:20

product-auto-label bot added size: m Pull request size is medium. size: xl Pull request size is extra large. and removed size: xl Pull request size is extra large. size: m Pull request size is medium. labels Aug 4, 2025

fix: race condition among transactions when running parallely

1f32e90

alkatrivedi force-pushed the fix-race-condition branch from b8ee5ed to 1f32e90 Compare August 4, 2025 09:11

product-auto-label bot added size: m Pull request size is medium. size: xl Pull request size is extra large. and removed size: xl Pull request size is extra large. size: m Pull request size is medium. labels Aug 4, 2025

alkatrivedi force-pushed the fix-race-condition branch from 035d1d2 to ab544ee Compare August 4, 2025 11:15

product-auto-label bot added size: m Pull request size is medium. and removed size: xl Pull request size is extra large. labels Aug 4, 2025

alkatrivedi force-pushed the fix-race-condition branch 2 times, most recently from 7382103 to 414cd05 Compare August 4, 2025 11:22

product-auto-label bot added size: xl Pull request size is extra large. and removed size: m Pull request size is medium. labels Aug 4, 2025

alkatrivedi force-pushed the fix-race-condition branch from e006a70 to bd150db Compare August 4, 2025 11:30

product-auto-label bot added size: m Pull request size is medium. size: xl Pull request size is extra large. and removed size: xl Pull request size is extra large. size: m Pull request size is medium. labels Aug 4, 2025

surbhigarg92 reviewed Aug 4, 2025

View reviewed changes

product-auto-label bot added size: xl Pull request size is extra large. and removed size: m Pull request size is medium. labels Aug 5, 2025

test: add mock spanner test

b645053

alkatrivedi force-pushed the fix-race-condition branch from 85133cd to b645053 Compare August 5, 2025 09:51

product-auto-label bot added size: m Pull request size is medium. and removed size: xl Pull request size is extra large. labels Aug 5, 2025

🦉 Updates from OwlBot post-processor

a446455

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

product-auto-label bot added size: xl Pull request size is extra large. and removed size: m Pull request size is medium. labels Aug 5, 2025

sakthivelmanii reviewed Aug 5, 2025

View reviewed changes

system-test/spanner.ts Show resolved Hide resolved

sakthivelmanii reviewed Aug 5, 2025

View reviewed changes

surbhigarg92 reviewed Aug 6, 2025

View reviewed changes

refactor test

6459b9e

alkatrivedi force-pushed the fix-race-condition branch from c164ecb to 6459b9e Compare August 6, 2025 11:34

surbhigarg92 reviewed Aug 7, 2025

View reviewed changes

test/spanner.ts Outdated Show resolved Hide resolved

test/spanner.ts Show resolved Hide resolved

test/mockserver/mockspanner.ts Outdated Show resolved Hide resolved

refactor mock spanner

a8738bd

surbhigarg92 reviewed Aug 8, 2025

View reviewed changes

refactor mockspanner

225a84d

alkatrivedi force-pushed the fix-race-condition branch from 914c308 to 225a84d Compare August 8, 2025 07:30

alkatrivedi added kokoro:force-run Add this label to force Kokoro to re-run the tests. owlbot:run Add this label to trigger the Owlbot post processor. labels Aug 8, 2025

gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Aug 8, 2025

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 8, 2025

surbhigarg92 approved these changes Aug 8, 2025

View reviewed changes

alkatrivedi merged commit f8b6f63 into main Aug 10, 2025
23 of 24 checks passed

alkatrivedi deleted the fix-race-condition branch August 10, 2025 09:55

release-please bot mentioned this pull request Jul 31, 2025

chore(main): release 8.2.0 #2359

Merged

		process.env.GOOGLE_CLOUD_SPANNER_MULTIPLEXED_SESSIONS = 'true';
		process.env.GOOGLE_CLOUD_SPANNER_MULTIPLEXED_SESSIONS_FOR_RW = 'true';

	seqNum: 2,
	// Retry should have a higher sequence number
	seqNum: 2,

fix: race condition among transactions when running parallely #2369

fix: race condition among transactions when running parallely #2369

Uh oh!

Conversation

alkatrivedi commented Aug 4, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants